Skip to content
This repository has been archived by the owner on Sep 27, 2024. It is now read-only.

Fix delete word not replacing text #833

Merged

Conversation

aringenbach
Copy link
Contributor

Should fix #808

I couldn't explore potential Web related corner cases so it might be better if a Web developer is available to double check this.

@codecov-commenter
Copy link

codecov-commenter commented Oct 2, 2023

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (90754f6) 88.38% compared to head (f2474c4) 89.91%.

Additional details and impacted files
@@             Coverage Diff              @@
##               main     #833      +/-   ##
============================================
+ Coverage     88.38%   89.91%   +1.52%     
============================================
  Files           157       82      -75     
  Lines         18104    14767    -3337     
  Branches        945        0     -945     
============================================
- Hits          16002    13278    -2724     
+ Misses         1853     1489     -364     
+ Partials        249        0     -249     
Flag Coverage Δ
uitests ?
uitests-android ?
uitests-ios ?
unittests 89.91% <100.00%> (+3.24%) ⬆️
unittests-android ?
unittests-ios ?
unittests-rust 89.91% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
crates/wysiwyg/src/composer_model/delete_text.rs 97.41% <100.00%> (ø)

... and 75 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@aringenbach aringenbach changed the title Fix delete backspace not replacing text Fix delete word backspace not replacing text Oct 2, 2023
@aringenbach aringenbach changed the title Fix delete word backspace not replacing text Fix delete word not replacing text Oct 2, 2023
@aringenbach aringenbach force-pushed the fix-delete-backward-not-replacing-text branch from c045bd4 to f2474c4 Compare October 2, 2023 11:40
@sonarcloud
Copy link

sonarcloud bot commented Oct 2, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@jonnyandrew jonnyandrew requested a review from a team October 3, 2023 10:22
@jonnyandrew
Copy link
Contributor

Thanks for the PR @aringenbach.

I couldn't explore potential Web related corner cases so it might be better if a Web developer is available to double check this.

I wonder if @florianduros / @andybalaam could help with this?

@florianduros
Copy link
Contributor

Thanks for the PR @aringenbach.

I couldn't explore potential Web related corner cases so it might be better if a Web developer is available to double check this.

I wonder if @florianduros / @andybalaam could help with this?

I tested on Macos (Option + Delete shortcut) and it doesn't work on multiple lines:

Enregistrement.de.l.ecran.2023-10-11.a.15.31.53.mov

@aringenbach
Copy link
Contributor Author

I tested on Macos (Option + Delete shortcut) and it doesn't work on multiple lines:

Yeah I actually tested a bit in the meantime (on Chrome & Safari for MacOS) and it looks like a few whacky things are happening with the delete word feature both with or without this change.
I think the Rust change here makes sense from the model perspective, but it seems like there might be some tidying up to do around the entire feature that might be out of my current level of expertise 🧐

@Velin92 Velin92 enabled auto-merge (squash) January 18, 2024 15:57
@Velin92 Velin92 merged commit f0d503e into matrix-org:main Jan 18, 2024
7 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Web editor not updating content after deleting a word (ctrl+backspace)
5 participants